-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
expose quality parameter as part of public API #6
base: master
Are you sure you want to change the base?
Conversation
|
So I will create a WebpEncoder struct which you can create with a custom quality_factor or not, and it will wrap the current webp_write methods instead of having them be freestanding functions. If it was "just" a custom function, I would have to duplicate every single function to have one variant which includes the parameter and one variant which doesn't. |
492c364
to
b5d6981
Compare
I also added that |
9ac73b6
to
a2defdd
Compare
C: Deref<Target = [u8]>, | ||
{ | ||
let buf = libwebp::WebPEncodeRGBA(&img, img.width(), img.height(), img.width() * 4, 75.0) | ||
pub struct WebpEncoder<W: Write> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it implement ImageEncoder?
Yes, it could; here's a proof of concept implementation. ColorType::L8 => {
let img = DynamicImage::ImageLuma8(
ImageBuffer::from_raw(width, height, buf.to_vec()).unwrap(),
);
self.encode_rgb(&img.to_rgb8())
}
ColorType::La8 => {
let img = DynamicImage::ImageLumaA8(
ImageBuffer::from_raw(width, height, buf.to_vec()).unwrap(),
);
self.encode_rgba(&img.to_rgba8())
} However this might be a hidden cost as we need an allocation ( I would like to document this behavior (and most of the library) in another commit if that's ok, but I can also do a new PR for that if you prefer. |
75fddd8
to
7dee4af
Compare
As you said, following JpegEncoder's behavior seems better.
The document would be helpful. Could you go ahead and provide one? |
this was previously always set to 0.75, but in case library users might want to use custom values for quality_factor, so I am exposing that with this PR. If you don't like my formatting, I can change it back to use longer lines, but then the code won't be rustfmt-formatted anymore (which it was before).
Related question: I will definitely also need a way to losslessly encode; would like to know how you would like to see that exposed in the API. I would probably change the quality_factor to an
enum Compression { Lossless, Lossy(f32) }
, but am open to other options. I could also add that new API to this PR if you'd like.